Skip to content

Declare documented API public (downstream ExplicitImports)#1498

Draft
ChrisRackauckas-Claude wants to merge 3 commits into
SciML:masterfrom
ChrisRackauckas-Claude:declare-public-documented
Draft

Declare documented API public (downstream ExplicitImports)#1498
ChrisRackauckas-Claude wants to merge 3 commits into
SciML:masterfrom
ChrisRackauckas-Claude:declare-public-documented

Conversation

@ChrisRackauckas-Claude

@ChrisRackauckas-Claude ChrisRackauckas-Claude commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Marks Catalyst's documented-but-not-exported API names as public (via the existing SciMLPublic.@public mechanism already used in src/Catalyst.jl), so they participate in Julia's public-symbol metadata. Exported names are already public; this PR targets only names that downstream packages reach via qualified access (Catalyst.name).

Rationale (downstream ExplicitImports)

Downstream SciML packages run ExplicitImports' check_all_qualified_accesses_are_public, which flags any Catalyst.name access where name is not declared public (and not exported). Several documented Catalyst accessors/predicates/types are accessed this way downstream but were neither exported nor @public, so they were flagged. Declaring them public resolves those findings without changing Catalyst's exported surface.

Selection criteria

A name was publicized only if it is (defined in src/) AND (documented — has a docstring and/or appears in docs/src, including @docs API-reference blocks) AND (not already exported, including via the @reexport using ModelingToolkitBase reexport) AND (not already @public) AND (Catalyst-owned, i.e. not re-imported from another module).

Names publicized (21)

Grouped next to their relevant include in src/Catalyst.jl:

  • Reaction predicates / metadata accessors: isbc, isconstant, isvalidreactant, hasnoisescaling, getnoisescaling
  • ReactionSystem / ReactionComplex types and accessors: ReactionComplex, ReactionComplexElement, get_species, get_rxs, get_networkproperties, reset_networkproperties!
  • Symbolic map helper: symmap_to_varmap
  • Network analysis: cycles
  • DiscreteSpaceReactionSystem accessors: reactionsystem, spatial_reactions, num_verts, num_edges, num_species, dspace, edge_iterator, is_transport_system

All explicitly-requested downstream-flagged candidates are included: symmap_to_varmap, get_networkproperties, cycles (all defined + documented + not exported).

Removed because already exported (4)

The first revision of this PR incorrectly declared four reaction-metadata accessors public. They are already exported into Catalyst's namespace via @reexport using ModelingToolkitBase (ModelingToolkitBase exports them), and Julia forbids declaring an already-exported name public:

ERROR: LoadError: cannot declare Catalyst.hasdescription public; it is already declared exported

Removed from the @public additions (verified exported by querying names(ModelingToolkitBase) and confirmed Base.isexported(Catalyst, n) == true after load):

  • hasdescription, getdescription, hasmisc, getmisc

These are public already (export ⟹ public), so downstream ExplicitListing checks are unaffected. The export-scan that produced the original list missed them because they are not exported in Catalyst's own src/ — they enter the namespace through the ModelingToolkitBase reexport.

Considered and skipped (with reasons)

  • assemble_oderhs — defined in src/reactionsystem_conversions.jl, but has no docstring and appears only in docs/unpublished/pdes.md (not in docs/src). Internal-only by the documentation criterion → skipped.
  • flatten — referenced in docs, but it is imported from ModelingToolkitBase (import ModelingToolkitBase: ... flatten ... in src/Catalyst.jl), i.e. not a Catalyst-owned name → skipped (publicity is the owning module's responsibility).
  • ModelingToolkitBase, SymbolicIndexingInterface — these are module references in @docs/docs, not Catalyst-owned API names → skipped.
  • Already-exported documented names (e.g. adjacencymat, deficiency, subnetworks, grid_dims, grid_size, and the bulk of the @docs list) — export already makes them public → not touched.
  • Already-@public names (unit-validation APIs, U0Map/ParameterMap and their accessors) — left as-is.

Verification

Loaded on Julia 1.12 (julia +1.12 --project=. -e 'using Catalyst'): precompiles and loads cleanly with no "already declared exported" error (CATALYST LOADED OK). Post-load Base.ispublic/Base.isexported confirm the 21 kept names are public + not exported, and the 4 removed names are exported (so correctly left undeclared).

Mechanism / dependency

No new dependency added. Catalyst already depends on SciMLPublic ([deps] + [compat] = "1") and already uses import SciMLPublic: @public with several @public blocks; this PR appends to that existing mechanism. Patch version bumped 16.2.016.2.1.


This PR should be ignored until reviewed by @ChrisRackauckas.

🤖 Generated with Claude Code

@ChrisRackauckas

Copy link
Copy Markdown
Member

@TorkelE @isaacsas any problem with this list?

Mark Catalyst's documented-but-not-exported names as `public` via the
existing `SciMLPublic.@public` mechanism, so downstream packages that
qualified-access these names (`Catalyst.name`) pass ExplicitImports'
check_all_qualified_accesses_are_public.

Only names that are defined in src, documented (docstring and/or docs/src),
and not already exported are publicized. Bumps patch version.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
`hasdescription`, `getdescription`, `hasmisc`, `getmisc` are exported
into Catalyst's namespace through `@reexport using ModelingToolkitBase`,
so declaring them `public` is a load error:

  cannot declare Catalyst.hasdescription public; it is already declared exported

The original export-scan only parsed Catalyst's own `src/` `export` lines and
missed reexported names. Remove the four reexport-exported names from the
`@public` additions; the remaining 21 documented, non-exported names stay.
Verified `using Catalyst` loads cleanly on Julia 1.12.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
The metadata-survives-complete() bug (issue SciML#1353) is fixed upstream in
ModelingToolkitBase: getmetadata(complete(rs2), MiscSystemData, nothing)
now correctly returns ones(2, 3). The @test_broken was reporting an
Unexpected Pass and erroring the Modeling test group. Verified locally on
Julia 1.12 that the assertion now holds.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
@ChrisRackauckas-Claude ChrisRackauckas-Claude force-pushed the declare-public-documented branch from 5a916d9 to bf48907 Compare July 5, 2026 17:27
@ChrisRackauckas-Claude

Copy link
Copy Markdown
Contributor Author

Rebase + CI status update (automated)

Rebased onto current master (4296acaf, "QA: run_qa v1.6 form"). Clean rebase, no conflicts; the branch's own diff stays minimal (Project.toml version bump + 18 lines of @public in src/Catalyst.jl). Verified on Julia 1.12: using Catalyst precompiles cleanly and all 21 @public names resolve as defined && public && not-exported.

Modeling lane red — two pre-existing, upstream-driven failures (not caused by this PR)

The @public additions are metadata-only and cannot alter runtime/solver behavior. The Modeling-group red is entirely pre-existing on master:

  1. test/reactionsystem_core/reactionsystem.jl:1150 — Unexpected Pass (FIXED here).
    The @test_broken for the MiscSystemData-survives-complete() bug (complete swaps order of metadata in ReactionSystems #1353) now passes because it's been fixed upstream in ModelingToolkitBase. It was erroring the whole Modeling group as an Unexpected Pass. Converted @test_broken@test (verified locally on 1.12: getmetadata(complete(rs2), MiscSystemData, nothing) == ones(2,3) is true). This is the same error that reddened master's own CI run 28646912105 — the group aborted here, which masked the second failure below.

  2. test/network_analysis/conservation_laws.jl — 8 failures (upstream-blocked, NOT fixed).
    Lines 150,152,179,183,287,290,325,327. These are conserved-quantity / SteadyStateProblem initialization divergences: nsol1 ≈ sssol1 fails because the SteadyStateProblem+DynamicSS path returns wrong values (incl. negative concentrations, e.g. -5.666) for conserved-species systems, while the NonlinearProblem+remove_conserved path is correct. The Γ/derived-species init-value checks (oprob[X2]==2.0, Γ[1]==3.0) fail for the same reason.

    Reproduced byte-identical on a clean, unmodified master checkout (Catalyst 16.2.0) in a standalone env (MTKBase 1.51.0, SciMLBase 3.31.0, DiffEqBase 7.6.0): 242 passed, 8 failed, 17 broken — same lines, same evaluated values. So this is an upstream ModelingToolkitBase/SciMLBase initialization regression (the let block even notes it depends on "MTK #4177 … now fixed" — it has regressed again). It only surfaces in CI now because fix Higher order terms in reaction rates #1 lets the group run past reactionsystem.jl.

    Per repo policy I did not loosen the tolerances or @test_broken these — that would hide a real upstream numerical bug. This needs an upstream MTKBase/SciMLBase conserved-quantity SteadyStateProblem init fix (or a maintainer decision) before the lane can go green.

The build/Documentation job red is also pre-existing on master (docs @example blocks: verbose::Bool no longer accepted in OrdinaryDiffEq v7) and is out of scope for this PR.

Net: the @public change itself is correct and verified; the lane cannot be driven fully green from this PR because of the pre-existing upstream conservation_laws SteadyStateProblem init bug. Ignore until reviewed by @ChrisRackauckas.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants